-
Notifications
You must be signed in to change notification settings - Fork 126
Add projection v1.2.0 to the list of schema URIs in projection.py #1590
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The migration of projection to latest version systematically pops the field epsg. If extension URI is pointing to versions 1.0.0 or 1.1.0, it is also changed to point to version 2.0.0. However, if schema URI is v1.2.0 the extension schema URI is not changed. I propose to change it as well in that case so that the change is explicit for all versions < 2.0.0. That would help finding out that a change occurred.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1590 +/- ##
==========================================
- Coverage 92.32% 92.30% -0.02%
==========================================
Files 55 55
Lines 8381 8387 +6
Branches 966 970 +4
==========================================
+ Hits 7738 7742 +4
Misses 457 457
- Partials 186 188 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gadomski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me, and could you add a simple regression test? The code in
pystac/tests/extensions/test_projection.py
Lines 577 to 594 in 065a631
| def test_migrate_item() -> None: | |
| old = "https://stac-extensions.github.io/projection/v1.1.0/schema.json" | |
| current = "https://stac-extensions.github.io/projection/v2.0.0/schema.json" | |
| path = TestCases.get_path("data-files/projection/example-with-version-1.1.json") | |
| item = Item.from_file(path) | |
| assert old not in item.stac_extensions | |
| assert current in item.stac_extensions | |
| assert item.ext.proj.epsg == 32614 | |
| assert item.ext.proj.code == "EPSG:32614" | |
| assert item.assets["B1"].ext.proj.epsg == 32614 | |
| assert item.assets["B1"].ext.proj.code == "EPSG:32614" | |
| assert item.assets["B8"].ext.proj.epsg == 9999 | |
| assert item.assets["B8"].ext.proj.code == "EPSG:9999" |
|
@gadomski I went to add a test and ended up adding a little logic to handle that v1.2.0 release jussssttt in case. |
|
@floriandeboissieu hope you don't mind that I pushed to your branch. This one looked close to done so I just added a test to get it all the way there. |
Description:
The migration of projection to latest version systematically pops the field epsg. If extension URI is pointing to versions 1.0.0 or 1.1.0, it is also changed to point to version 2.0.0. However, if schema URI is v1.2.0, the extension schema URI is not updated. Although it is not necessary to validate items as both properties
proj:codeandproj:epsgare authorized in that v1.2.0, I propose to update it as well to v2.0.0 when migrating so that the change is explicit for all versions < 2.0.0. That would help finding out that a change occurred without having to inspect all assets.PR Checklist:
pre-commit run --all-files)pytest)